WIP `Query`.`fetch_raw` #182

slvrtrn · 2024-11-28T12:36:43Z

Summary

Adds a method to fetch raw bytes from a query, exposes RawCursor, and adds an additional cursor type (RawCursorNewline) to work with arbitrary formats where a newline character separates rows. See the related examples regarding streaming into a file, as well as streaming separate *EachRow records.

Checklist

Unit and integration tests covering the common scenarios were added
A human-readable description of the changes was provided so that we can include it in CHANGELOG later
For significant changes, documentation in README and https://github.com/ClickHouse/clickhouse-docs was updated with further explanations or tutorials

pravic · 2024-11-28T17:17:15Z

examples/raw_stream_json_each_row.rs

+        .query(
+            "
+                SELECT number, hex(randomPrintableASCII(20)) AS hex_str
+                FROM system.numbers
+                LIMIT 10
+                FORMAT JSONEachRow
+            ",
+        )
+        // By default, ClickHouse quotes (U)Int64 in JSON* family formats;
+        // disable it to simplify this example.
+        .with_option("output_format_json_quote_64bit_integers", "0")
+        .fetch_raw()?
+        .newline();


Maybe like this?

Suggested change

.query(

"

SELECT number, hex(randomPrintableASCII(20)) AS hex_str

FROM system.numbers

LIMIT 10

FORMAT JSONEachRow

",

)

// By default, ClickHouse quotes (U)Int64 in JSON* family formats;

// disable it to simplify this example.

.with_option("output_format_json_quote_64bit_integers", "0")

.fetch_raw()?

.newline();

.query(

"

SELECT number, hex(randomPrintableASCII(20)) AS hex_str

FROM system.numbers

LIMIT 10

",

)

// By default, ClickHouse quotes (U)Int64 in JSON* family formats;

// disable it to simplify this example.

.with_option("output_format_json_quote_64bit_integers", "0")

.with_format("JSONEachRow")

.fetch_raw()?

.newline();

Because specifying FORMAT xxx as part of the SQL feels not convenient and a bit more error prone.

Ideally, I would prefer with_format(clickhouse::Format::JsonEachRow) if the maintenance burden of mapping all formats to an enum isn't too much.

This is possible after a few tweaks to the SQLBuilder. See this commit.

Let's leave the raw API as-is. The user will quickly find an invalid FORMAT with the first query.

Same thread #182 (comment)

slvrtrn · 2024-12-04T16:29:15Z

Open questions so far:

Error handling. This test returns an error locally, but the contents are off:

Err(BadResponse("{\"s\":0}\n{\"exception\": \"Code: 159. DB::Exception: Timeout exceeded: elapsed 0.100752804 seconds, maximum: 0.08. (TIMEOUT_EXCEEDED) (version 24.10.1.2812 (official build))\"}"))

Fails on the CI for some reason.

This API probably has the same issue as The fetch API is unsound if used with borrowed types #24
Output formats. Currently, it is parsed from the docs, and every format that is suitable for the output is included. Shall we keep all of them, or only a select subset?

loyd · 2024-12-05T09:59:39Z

examples/raw_stream_json_each_row.rs

+            ",
+        )
+        // By default, ClickHouse quotes (U)Int64 in JSON* family formats;
+        // disable it to simplify this example.


to simplify this example

because serde_json supports u64 natively, there is no reason to use strings here

loyd · 2024-12-05T10:06:55Z

examples/raw_stream_json_each_row.rs

+
+// NB: there is no `Row` derive here
+#[derive(Debug, Deserialize)]
+struct MyRowInJSONEachRowFormat {


I suggest avoiding any use cases where using non-raw API is preferred.

Here we can use https://docs.rs/serde_json/latest/serde_json/enum.Value.html

loyd · 2024-12-05T10:07:39Z

src/cursor.rs

@@ -13,7 +14,7 @@ use crate::{

 // === RawCursor ===

-struct RawCursor(RawCursorInner);
+pub struct RawCursor(RawCursorInner);


docs (preparing for warn(missing_docs) lint).

loyd · 2024-12-05T10:11:04Z

src/format.rs

+use std::fmt;
+
+#[derive(Debug, Clone)]
+pub enum OutputFormat {


It's probably better to leave this to the user. This format seems useless for us and will require extending this enum in the future.

Btw: all public enums should be annotated with #[non_exhaustive] (for further compatible extensions).

On the other hand, we can use this info to better understand errors (like sent inside payload in JSON case), but probably there more seamless way

loyd · 2024-12-05T10:17:04Z

tests/it/fetch_raw.rs

+use clickhouse::format::OutputFormat;
+
+#[tokio::test]
+async fn fetch_raw_with_error() {


What about testing a successful path?

loyd · 2024-12-05T10:18:39Z

src/lib.rs

@@ -11,7 +11,9 @@ use std::{collections::HashMap, fmt::Display, sync::Arc};
 pub use self::{compression::Compression, row::Row};
 pub use clickhouse_derive::Row;

+pub mod cursor;


It seems to be implementation details; let's reexport from mod query where this is actually used.

loyd · 2024-12-05T10:31:02Z

src/cursor.rs

@@ -27,11 +28,18 @@ struct RawCursorLoading {
 }

 impl RawCursor {
-    fn new(response: Response) -> Self {
+    pub(crate) fn new(response: Response) -> Self {
        Self(RawCursorInner::Waiting(response.into_future()))
    }



Probably, we should also add into_futures03(self) -> RawStream03 (implementing https://docs.rs/futures/latest/futures/prelude/trait.Stream.html) under a new futures03 feature (to support future v1 and AsyncIterator from std when they will released).

This is de facto the standard for streams (with built-in combinators, e.g. concat) and should be used for owned types (like Bytes).

Alternatively, we can directly implement Stream for RawCursor (also under the feature).

loyd · 2024-12-05T10:32:19Z

examples/raw_stream_json_each_row.rs

@@ -0,0 +1,54 @@
+use std::str::from_utf8;


Another real use case is to collect everything into one Bytes (or Vec<u8>). It's nice to have such an example, too!

loyd · 2024-12-05T10:36:57Z

src/cursor.rs

        Self(RawCursorInner::Waiting(response.into_future()))
    }

-    async fn next(&mut self) -> Result<Option<Bytes>> {
+    pub fn newline(self) -> RawCursorNewline {


The futures crate already provides the AsyncBufReadExt::lines method.

It seems we can implement the AsyncBufRead (under the futures03 feature) trait for RawCursor covering more use cases, for instance the use of io::copy. This function can be also used in the example of streaming to a file.

loyd · 2024-12-05T10:47:57Z

examples/raw_stream_into_file.rs

+
+    let mut file = File::create(filename).await.unwrap();
+
+    loop {


See #182 (comment) for how to make this example more canonical.

slvrtrn added 2 commits November 8, 2024 18:01

Add Query::fetch_raw, expose RawCursor

bad2ea3

Raw streaming with newline support draft

b85f17e

slvrtrn mentioned this pull request Nov 28, 2024

Fetch rows in JSON format #152

Open

3 tasks

pravic reviewed Nov 28, 2024

View reviewed changes

slvrtrn added 4 commits December 4, 2024 15:23

Merge remote-tracking branch 'origin/main' into fetch_raw

697e719

OutputFormat enum, move FORMAT clause to the sql builder

cb22c7d

Remove useless match

0b01dc3

fetch_raw_with_error fix

569519d

loyd requested changes Dec 5, 2024

View reviewed changes

loyd reviewed Dec 5, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP `Query`.`fetch_raw` #182

WIP `Query`.`fetch_raw` #182

slvrtrn commented Nov 28, 2024

pravic Nov 28, 2024

pravic Nov 28, 2024 •

edited

Loading

slvrtrn Dec 4, 2024

loyd Dec 5, 2024 •

edited

Loading

slvrtrn commented Dec 4, 2024

loyd Dec 5, 2024

loyd Dec 5, 2024

loyd Dec 5, 2024 •

edited

Loading

loyd Dec 5, 2024

loyd Dec 8, 2024

loyd Dec 5, 2024

loyd Dec 5, 2024

loyd Dec 5, 2024 •

edited

Loading

loyd Dec 5, 2024

loyd Dec 5, 2024 •

edited

Loading

loyd Dec 5, 2024

WIP Query.fetch_raw #182

Are you sure you want to change the base?

WIP Query.fetch_raw #182

Conversation

slvrtrn commented Nov 28, 2024

Summary

Checklist

Choose a reason for hiding this comment

pravic Nov 28, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

loyd Dec 5, 2024 • edited Loading

Choose a reason for hiding this comment

slvrtrn commented Dec 4, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

loyd Dec 5, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

loyd Dec 5, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

loyd Dec 5, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

WIP `Query`.`fetch_raw` #182

WIP `Query`.`fetch_raw` #182

pravic Nov 28, 2024 •

edited

Loading

loyd Dec 5, 2024 •

edited

Loading

loyd Dec 5, 2024 •

edited

Loading

loyd Dec 5, 2024 •

edited

Loading

loyd Dec 5, 2024 •

edited

Loading